Repository navigation
[No QA][TS migration] Migrate 'ReportActionsUtilsTest.js' test to TypeScript - #36482
Conversation
b0851f4 to
45d4052
Compare
| * This is all handled with getSortedReportActions() which is used by several other methods to keep the code DRY. | ||
| */ | ||
| function getSortedReportActionsForDisplay(reportActions: ReportActions | null, shouldMarkTheFirstItemAsNewest = false): ReportAction[] { | ||
| function getSortedReportActionsForDisplay(reportActions: ReportActions | ReportAction[] | null, shouldMarkTheFirstItemAsNewest = false): ReportAction[] { |
There was a problem hiding this comment.
will this function work when we pass an array?
There was a problem hiding this comment.
are we changing it because of test?
There was a problem hiding this comment.
if yest I would adjust tests to pass correct types instead of changing a type here @fabioh8010 WYDT?
There was a problem hiding this comment.
Yes it works as expected with array (no diff between arrays and objects, since we are still dealing with key values pairs)
Ollyws
left a comment
There was a problem hiding this comment.
Just a change to unit tests so doesn't require C+ review.
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #25281 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
| key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`, | ||
| key: 'reportActions_test', |
There was a problem hiding this comment.
Why this change, to signal it's a test since the key doesn't really matter here?
There was a problem hiding this comment.
It was changed to pass the ts check since with the template string it is not recognizable as a valid key for Onyx
There was a problem hiding this comment.
@pasyukevich I just checked out your PR here, and it's working fine with ${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}, could you re-check?
There was a problem hiding this comment.
@fabioh8010 reverted
after changes in pr this update became redundant
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / SafariMacOS: DesktopNote: checklist was filled out for GH action to succeed, but not completed since this just changes a test file |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/cead22 in version: 1.4.44-0 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 1.4.44-13 🚀
|
Details
Fixed Issues
$ #25281
PROPOSAL:
Tests
Run unit tests and check that it finished successfully
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel so the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop